Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update images, add Kubernetes 1.20, prepare for v1.5.0 release #238

Merged
merged 2 commits into from
Jan 27, 2021

Conversation

pohly
Copy link
Contributor

@pohly pohly commented Jan 27, 2021

What type of PR is this?
/kind cleanup

What this PR does / why we need it:

Kubernetes 1.17 is no longer supported, but because all sidecars still
work on it there is no need to remove the "deploy/kubernetes-1.17"
folder either. Instead, all sidecar and driver images merely get
updated.

The CSI hostpath image update is important because the recently added
health check sidecar depends on the new version.

Kubernetes 1.20 is where officially the v1 snapshotter API is
introduced. Therefore the new deployment for it also uses the
external-snapshotter 4.x and the v1 APIs.

** Note for reviewers ***

I am including a CHANGELOG/CHANGELOG-1.5.md in this PR for the sake of expedience. It already includes this PR, so once it is merged, we can tag v1.5.0.

Does this PR introduce a user-facing change?:

image updates and deployment for Kubernetes 1.20 with external-snapshotter v4.0.0.

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 27, 2021
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 27, 2021
@pohly
Copy link
Contributor Author

pohly commented Jan 27, 2021

/retest

Test flake (?) on 1.20: "External Storage [Driver: hostpath.csi.k8s.io] [Testpattern: Dynamic PV (default fs)] subPath should support restarting containers using directory as subpath [Slow]"

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 27, 2021
@pohly pohly changed the title deploy: update images, add Kubernetes 1.20 update images, add Kubernetes 1.20, prepare for v1.5.0 release Jan 27, 2021
@@ -91,7 +91,7 @@ spec:
name: csi-data-dir

- name: hostpath
image: k8s.gcr.io/sig-storage/hostpathplugin:v1.4.0
image: k8s.gcr.io/sig-storage/hostpathplugin:v1.5.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will CI start to fail for other repos if we merge this before releasing hostpathplugin v1.5.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Those other pre-merge jobs intentionally use a fixed version of the hostpath repo to prevent such breakage.

What normally would have happened is that the periodic jobs (briefly) fail until the new image is available. But those jobs were already broken when adding the health check sidecar, so this is not going to make them worse 😅

## Changes by Kind

### Feature
- Adds snapshot beta CRD deployment for 1.17 ([#98](https://github.com/kubernetes-csi/csi-driver-host-path/pull/98), [@xing-yang](https://github.com/xing-yang))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We deploy snapshot CRDs in prow, not the hostpath driver right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was another PR that doesn't belong here.


### Feature
- Adds snapshot beta CRD deployment for 1.17 ([#98](https://github.com/kubernetes-csi/csi-driver-host-path/pull/98), [@xing-yang](https://github.com/xing-yang))
- Ephemeral inline volumes in addition to normal volumes on Kubernetes 1.16 ([#97](https://github.com/kubernetes-csi/csi-driver-host-path/pull/97), [@pohly](https://github.com/pohly))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been available since 1.2.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the release-notes tool is confused by the release-tools subtree commit messages and picking up csi-release-tools PR numbers. Does the new method of squashing fix this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Darn. I did check one PR which looked odd and that turned out to be correct, so I trusted the tool for the rest 😢

I suppose squashing helps, but I haven't tried that myself - I don't do releases that often.


### Bug or Regression
- Add topology to the CreateVolumeResponse for existing volumes ([#231](https://github.com/kubernetes-csi/csi-driver-host-path/pull/231), [@Jiawei0227](https://github.com/Jiawei0227))
- Fixed raw block volumes on hosts that don't have /dev/loop- devices pre-defined ([#109](https://github.com/kubernetes-csi/csi-driver-host-path/pull/109), [@pohly](https://github.com/pohly))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is old?


### Uncategorized
- Build with Go 1.15 ([#198](https://github.com/kubernetes-csi/csi-driver-host-path/pull/198), [@pohly](https://github.com/pohly))
- Migrated to Go modules, so the source builds also outside of GOPATH. ([#103](https://github.com/kubernetes-csi/csi-driver-host-path/pull/103), [@pohly](https://github.com/pohly))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

old?

@@ -1,4 +1,4 @@
apiVersion: storage.k8s.io/v1beta1
apiVersion: storage.k8s.io/v1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v1 is only available in 1.18

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So shall we rename kubernetes-1.17 to kubernetes-1.18 after all?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done and force pushed (because it is a fix for the incorrect use of v1 in kubernetes-1.17).

@pohly
Copy link
Contributor Author

pohly commented Jan 27, 2021

I've pushed a version where I manually verified each PR that gets listed in the changelog.

pohly added 2 commits January 27, 2021 21:10
Kubernetes 1.17 is no longer supported and by updating to 1.18 as
minimal version we can use the GA CSIDriver v1 API.

Also, all sidecar and driver images get updated. The CSI hostpath
image update is important because the recently added health check
sidecar depends on the new version.

Kubernetes 1.20 is where officially the v1 snapshotter API is
introduced. Therefore the new deployment for it also uses the
external-snapshotter 4.x and the v1 APIs.
# are needed only because of condition explained in
# https://github.com/kubernetes/kubernetes/issues/69608

kind: Service
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did we miss removing these as part of #196?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably.

volumeMounts:
- name: socket-dir
mountPath: /csi
- name: csi-external-health-monitor-controller
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting the controller here seems inconsistent with how we currently have other controller sidecars in separate manifests. I guess we didn't really conclude how we want to go in #192

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That came from #210. I just copied it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I agree that it seems inconsistent. I don't know whether it had to be done this way.

@msau42
Copy link
Collaborator

msau42 commented Jan 27, 2021

/lgtm
/approve

Let's followup on these cleanup items separately.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 27, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msau42, pohly

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 27, 2021
@k8s-ci-robot k8s-ci-robot merged commit 78a7533 into kubernetes-csi:master Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants